-
Notifications
You must be signed in to change notification settings - Fork 0
fix deployment for cloudscale-metrics-collector #23
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think you reference the correct secret
66ed155
to
9e8fdb0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM now I think. You might want to enable multi instancing support if we need to instance this exporter for every zone. See: https://syn.tools/commodore/reference/architecture.html#_component_instantiation
Maybe someone who's more familiar with the architecture should take a look at this too though
this is enabled in the exoscale metrics collector but I don't really understand it tbh. Probably because I'm not familiar enough with commodore and our deployment setup. Not sure if @Kidswiss knows more if we need this or not? |
@mweibel This change to the metrics collector is so, that each instance will only handle exactly one cluster where the cloudscale appcat services are deployed. We currently have two clusters where this is the case, appuio cloudscale and appuio exoscale. So in order to get the metrics of all the clusters, we would need two instances of this deployed on the appuio cloudscale cluster (where the database currently lives). I know that's a rather suboptimal approach, but once we have an actual API for the billing, we can then deploy the collectors to each cluster. |
9e8fdb0
to
69910ac
Compare
added multi instance - hope that's done the right way? |
69910ac
to
578e872
Compare
ah wait, nvm - still need another change. |
3dc877a
to
2bcceda
Compare
ok all good now. |
2bcceda
to
7b5dfea
Compare
7b5dfea
to
656d71e
Compare
after discussion with @zugao we decided to keep image v0.4.1 as the version to deploy for the image and first test the multi instance deployment. This also ensures when merging this and updating it in the catalog, we can test first multi instance deployment and only afterwards the k8s API changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Summary
Adds missing update for new configuration options since #22 got merged.
Checklist
bug
,enhancement
,documentation
,change
,breaking
,dependency
as they show up in the changelog